From 1b354ce174983eab7c4885325b7e04c45edcd0c3 Mon Sep 17 00:00:00 2001 From: "kaf24@freefall.cl.cam.ac.uk" Date: Mon, 1 Nov 2004 10:00:03 +0000 Subject: [PATCH] bitkeeper revision 1.1159.1.319 (41860923CuMAB3frY4t4g-Ls_iqqzg) Clean up some Xen comments to clarify execution order w.r.t. TLB flushes. --- xen/arch/x86/flushtlb.c | 12 ++++++++++-- xen/arch/x86/memory.c | 9 +++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/xen/arch/x86/flushtlb.c b/xen/arch/x86/flushtlb.c index 6e50febc4f..2079bf51c6 100644 --- a/xen/arch/x86/flushtlb.c +++ b/xen/arch/x86/flushtlb.c @@ -41,7 +41,7 @@ void write_cr3(unsigned long cr3) t = tlbflush_clock; do { t1 = t2 = t; - /* Clock wrapped: someone else is leading a global TLB shootodiown. */ + /* Clock wrapped: someone else is leading a global TLB shootdown. */ if ( unlikely(t1 == 0) ) goto skip_clocktick; t2 = (t + 1) & WRAP_MASK; @@ -60,7 +60,15 @@ void write_cr3(unsigned long cr3) __asm__ __volatile__ ( "mov"__OS" %0, %%cr3" : : "r" (cr3) : "memory" ); /* - * STEP 3. Update this CPU's timestamp. + * STEP 3. Update this CPU's timestamp. Note that this happens *after* + * flushing the TLB, as otherwise we can race a NEED_FLUSH() test + * on another CPU. (e.g., other CPU sees the updated CPU stamp and + * so does not force a synchronous TLB flush, but the flush in this + * function hasn't yet occurred and so the TLB might be stale). + * The ordering would only actually matter if this function were + * interruptible, and something that abuses the stale mapping could + * exist in an interrupt handler. In fact neither of these is the + * case, so really we are being ultra paranoid. */ tlbflush_time[smp_processor_id()] = t2; diff --git a/xen/arch/x86/memory.c b/xen/arch/x86/memory.c index 1d390933a1..410829cc72 100644 --- a/xen/arch/x86/memory.c +++ b/xen/arch/x86/memory.c @@ -1593,7 +1593,7 @@ void ptwr_flush(const int which) MEM_LOG("ptwr: Could not read pte at %p\n", ptep); /* * Really a bug. We could read this PTE during the initial fault, - * and pagetables can't have changed meantime. XXX Multi-proc guests? + * and pagetables can't have changed meantime. XXX Multi-CPU guests? */ BUG(); } @@ -1620,13 +1620,14 @@ void ptwr_flush(const int which) MEM_LOG("ptwr: Could not update pte at %p\n", ptep); /* * Really a bug. We could write this PTE during the initial fault, - * and pagetables can't have changed meantime. XXX Multi-proc guests? + * and pagetables can't have changed meantime. XXX Multi-CPU guests? */ BUG(); } /* Ensure that there are no stale writable mappings in any TLB. */ - __flush_tlb_one(l1va); + /* NB. INVLPG is a serialising instruction: flushes pending updates. */ + __flush_tlb_one(l1va); /* XXX Multi-CPU guests? */ PTWR_PRINTK("[%c] disconnected_l1va at %p now %08lx\n", PTWR_PRINT_WHICH, ptep, pte); @@ -1766,7 +1767,7 @@ int ptwr_do_page_fault(unsigned long addr) { nl2e = mk_l2_pgentry(l2_pgentry_val(*pl2e) & ~_PAGE_PRESENT); update_l2e(pl2e, *pl2e, nl2e); - flush_tlb(); + flush_tlb(); /* XXX Multi-CPU guests? */ } /* Temporarily map the L1 page, and make a copy of it. */ -- 2.30.2